Repository navigation
.NET: fix(dotnet): persist workflow scope aliases consistently - #9749
Vincent Biret (baywet) wants to merge 12 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d5103860-77a3-4006-90bc-8a9770ea808b
There was a problem hiding this comment.
🟢 Approval recommended
The focused normalization changes have targeted regression coverage and no unresolved blocking findings.
No open issues 🎉
What changed in this PR
This PR aligns persisted state with live formula state in .NET declarative workflows so equivalent scope names consistently address the same values.
Changes:
- Canonicalizes managed scopes for writes, reads, initialization, and key enumeration while preserving custom and unspecified scope behavior.
- Adds regression coverage for alias overwrites, clearing, sensitivity restoration, and state access.
| File | Description |
|---|---|
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/PowerFx/WorkflowFormulaStateTests.cs |
Tests restoration after alias overwrites and clears, including sensitivity metadata. |
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Kit/IWorkflowContextExtensionsTests.cs |
Tests canonical scope routing for reads, initialization, and key enumeration. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowContext.cs |
Normalizes managed scope names using the existing resolver. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 0313abbb6155
Model: gpt-5.6-sol
Overview
The change consistently canonicalizes new Topic state access and persistence to Local, with focused coverage for reads, initialization, overwrite, sensitivity, clear, and restore behavior. Custom and null scopes remain pass-through, and managed-scope restrictions remain intact. However, persisted checkpoints created by earlier versions can contain physical Topic entries, and the new read redirection makes those entries unreachable without migration compatibility.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowContext.cs
Code coverage overviewLanguages: C# C# / code-coverage/dotnet
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d5103860-77a3-4006-90bc-8a9770ea808b
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: Findings reported
Scope: 1 net-new commit(s): 41207a50592c
Model: gpt-5.6-sol
Overview
The change consistently aliases managed Topic access to Local, adds explicit legacy migration and conflict rejection, preserves native portable types, and resets initialization after failed restores. The pre-write conflict check and decode-before-write staging are useful guardrails. Two migration lifecycle gaps remain: the migration assumes ownership of the run-wide shared Topic scope, and its queued updates make an otherwise supported immediate subsequent restore fail.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PowerFx/WorkflowFormulaState.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d5103860-77a3-4006-90bc-8a9770ea808b
|
Addressed the additional Copilot review-summary feedback about quadratic pending-update scans in 8ca1b34. Point operations now use direct PortableValue probes for Topic, ownership, sensitivity metadata, and Local instead of enumerating scopes. The plain/wrapped regression cases each perform 32 write/read pairs: full-scope enumerations dropped from 64 to zero, with bounded key-probe counts. Whole-state restoration and read-only snapshots retain their necessary scope enumeration. |
There was a problem hiding this comment.
🔵 Needs a closer look
Versioned checkpoint ownership changes durable-storage compatibility and publication/rollback semantics, requiring maintainer review of ADR 0048’s mixed-runtime and custom-checkpoint-store compatibility policy.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
|
Acknowledged the latest Copilot review on 8ca1b34: there are no open code findings, and the durable-storage compatibility policy needs maintainer review. Proposed ADR 0048 explicitly covers mixed-runtime limitations and preserving the private metadata scopes in custom checkpoint stores. The requested human decider, westey-m, was added as a reviewer at 2026-10-28T00:07:57Z. Human review remains pending; the ADR will stay proposed until the decider approves it. |
There was a problem hiding this comment.
MAF Automated Review — Iteration 8
Result: No findings
Scope: 1 net-new commit(s): 8ca1b347d308
Model: gpt-5.6-sol
Overview
This increment adds versioned canonical ownership metadata, replaces full-scope point probes with key-level reads, and preserves separate raw Topic and canonical Local values across publication and restore. The strongest guardrails are the shared ownership write/read choke points, fail-closed version handling, read-only snapshot restoration, and broad mixed fan-out, migration, clear, and fresh-checkpoint tests. After reconciling the proposed candidates against the previous head, supplied diff, and existing feedback, no Critical, High, or Medium defect was established as introduced by this review range.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 73fd14a3-691a-49bc-81cc-1e18e27b0c39
There was a problem hiding this comment.
🟡 Changes recommended
Both initialization branches omit collision validation and sensitivity/ownership metadata, allowing mixed fan-out writes that cause subsequent managed assignments to fail the code-first collision check.
1 open issue
Review effort: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 73fd14a3-691a-49bc-81cc-1e18e27b0c39
There was a problem hiding this comment.
🟡 Changes recommended
Legacy integer migration persists converted or nested values instead of the original payload, causing subsequent native integer reads to return defaults rather than the saved value.
2 open issues
1 resolved since last review
Review effort: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 73fd14a3-691a-49bc-81cc-1e18e27b0c39
There was a problem hiding this comment.
🔵 Needs a closer look
The versioned checkpoint ownership format and restoration publication/rollback boundary require maintainer review of ADR 0048, upgrade compatibility, and effects on custom checkpoint stores and executor callbacks.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 9
Result: No findings
Scope: 18 net-new commit(s): 45ea037d474b, a9f29d0e324b, 14ce83b5635b, 63d0998c9e3a, 10a94cf736d7, 3a70a81a260b, abaf33a3c8eb, 61357e4950a2, 981f70e3c7bb, 5a950ad5d3ea, 6afcecd428dd, 20458faef1e5, 4a272d2b8e96, 9b72ed4f96bc, cb2c029705fc, fc62baa2dfd5, 2bb948e749e2, 56f5385b56b4
Model: gpt-5.6-sol
Overview
This increment completes managed-state initialization migration by preserving native payloads, carrying sensitivity, recording canonical ownership, and deleting only identified legacy entries. Collision preflight, versioned ownership, executor-private pending updates, restoration rollback, and focused native/formula tests constrain the plausible failure paths. No supported Critical, High, or Medium issue remains after reconciliation.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.


Motivation & Context
Declarative workflow scope aliases should retain the latest assignment consistently
when state is read, initialized, cleared, or restored.
Description & Review Guide
state updates, native reads, initialization, and key enumeration. Reuse the
existing scope-name resolver and add round-trip and state-access coverage.
Migrate unambiguous legacy
Topicentries identified by declarative metadataand delete only those entries from the old scope. Reject overlapping entries
before migration writes, and retry failed
restores without skipping validation. Share the existing portable decoder so
migration preserves native storage types rather than coercing numbers.
Publish restoration updates before post-load hooks or resumed handlers run.
Register interpreter executors for the same shared restoration phase used by
Kit executors. Canonicalize aliases in the plain-context sensitivity helpers,
including state subsequently accessed through expression evaluation.
Without a formula-state restoration callback, plain helper reads fall back
to the requested metadata-marked legacy entry without modifying it; writes
remove only that entry after queuing its canonical value and metadata.
Ad hoc expression loading uses a read-only formula snapshot instead of queuing
migration from each reader. Share the collision preflight between plain and
wrapped assignments, rejecting ambiguous visible same-key
Topicentriesbefore any value, sidecar, or live formula update. Record separate, versioned
canonical ownership with Local assignments and migrated values, so concurrent
raw Topic and canonical Local writes do not misclassify unrelated state.
Use key-level PortableValue probes instead of enumerating scopes for each
point operation. Plain helper null updates remove sensitivity and ownership
metadata together with the value. Preserve both Local and Topic formula names
when binding the shared canonical state.
Document the storage decision in proposed ADR 0048.
If callbacks or restoration publication fail, discard queued updates and
re-import the checkpoint's persisted state before propagating the original
error. Index retrieval and post-load hook failures remain outside that rollback
boundary. Once both restore tasks finish, those later failures discard
unpublished hook updates without reverting successfully published state.
persisted state. Custom scopes keep their existing behavior; omitted declarative
scopes use
Local.Older checkpoints with an identified declarative key in both physical scopes
and no canonical ownership metadata require explicit reconciliation or a fresh
session instead of an inferred assignment order.
Unmarked
Topicentries remain untouched, preserving code-first state in mixedworkflows. Unmarked legacy declarative entries require explicit reconciliation.
Immediate repeated restores do not leave pending migration updates behind.
Failed restoration publication can be retried without retaining partially
applied writes. A later index or hook failure does not undo successful
publication and leave it inconsistent with live executor state.
Queued hook updates from a failed restore do not survive into an immediate retry.
Fan-out expression readers do not queue competing migration writes. Plain
helper clears do not leave metadata that could claim a later code-first key.
New canonical ownership permits independently owned raw and canonical values
to coexist after fan-out publication and restoration; older ambiguous entries
still reject. Point-helper lookup work stays bounded as pending writes grow.
Public API signatures are unchanged.
state and persisted state, including alias overwrite and clear operations.
Commands were run from
dotnet/.dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*CanonicalOwnership*' --no-progressEight mixed fan-out cases became unreadable, two migrated/raw-reuse cases
failed restoration, four updates/clears rejected independently owned state,
and two point-operation cases each enumerated full scopes 64 times.
unsupported-version cases. Coverage includes both execution modes,
plain/wrapped assignments, explicit None/Sensitive metadata, fresh JSON restore,
expression aliases, migration, clear/reuse, and bounded point lookups.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --no-progressdotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*LegacyBoundary*' --no-progressconflicting migration writes, six plain/wrapped collision writes were
accepted, and four clear/key-reuse cases claimed unrelated code-first values.
Previous read-only helper coverage now expects expression-only reads to
retain legacy storage; explicit writes and registered pre-hook restoration
still perform cleanup.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*PlainLegacyHelpers*' --no-progressmissed saved values, four writes left overlapping entries, and two ambiguous
reads/writes did not reject the overlap.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*Checkpoint_Restore_LaterFailureDiscardsHookUpdatesAsync' --no-progressdotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*PlainSensitivityAliasesRemainConsistentAsync' --no-progressafter migration and four encountered overlapping entries on another
restoration or expression evaluation.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*Checkpoint_Restore_LaterFailurePreservesPublishedStateAsync' --no-progressfailures reverted persisted values while live executor state stayed restored.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*ResumeAsync_LegacyCheckpoint*' --no-progressassignments encountered conflicting updates; two immediate repeated restores
rejected pending migration writes.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*Checkpoint_Restore_FailedPublicationCanBeRetriedAsync' --no-progresswrites after failure; two subsequent restores rejected queued updates.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --filter-method '*RestoreAsync_PreservesLatestAliasAssignmentAsync' --no-progressrestored state returned the earlier value instead of the latest assignment.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-class '*WorkflowFormulaStateTests' --no-progressDirectly seeded legacy values were not restored or cleaned up, and overlapping
entries were not rejected.
alias-assignment regression and repeated conflict rejection.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --no-progressnet10.0andnet472; no failures or skipsin the final run. An earlier run encountered an existing off-thread timing
failure in
RestoreCheckpointAsync_ReloadsFormulaSessionOnEveryLoadAsync;the same test failure was reproduced with implementation files restored to
the published
41207a505revision before returning to the final implementation.dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*RestoreAsync_LegacyMigrationPreservesNativeTypesAsync' --no-progresspassed case because migrated numeric storage types changed.
dotnet format src\Microsoft.Agents.AI.Workflows.Declarative\Microsoft.Agents.AI.Workflows.Declarative.csproj --no-restore --verbosity quietdotnet format src\Microsoft.Agents.AI.Workflows\Microsoft.Agents.AI.Workflows.csproj --no-restore --verbosity quietdotnet format tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests.csproj --no-restore --verbosity quietdotnet format tests\Microsoft.Agents.AI.Workflows.UnitTests\Microsoft.Agents.AI.Workflows.UnitTests.csproj --no-restore --verbosity quietdotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*ResumeAsync_PublishesLegacyMigration*' --no-progressconflicting updates at publication, and two observed missing migrated state
in another executor's post-load hook.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*ResumeAsync_PreservesUnmarkedTopicEntriesAsync' --no-progressunrelated unmarked entries as conflicting aliases.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-class '*WorkflowFormulaStateTests' --no-progressclearing, post-load visibility, immediate repeated restores, and preservation
of unrelated code-first entries.
dotnet test --project tests\Microsoft.Agents.AI.Workflows.UnitTests --no-build --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --no-progressnet10.0andnet472; no failures or skips.dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net472 --no-build --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --filter-method '*RestoreCheckpointAsync_ReloadsFormulaSessionOnEveryLoadAsync' --no-progresschanged live state before the test's immediate assertions. Implementation
equality with
41207a505was verified before testing. The existing test wasnot weakened or changed.
dotnet build agent-framework-dotnet.slnx --disable-build-servers -nr:false --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --no-restore --tl:off --verbosity minimal -m:2After the initial build exhausted the repository drive and was stopped,
$artifactswas placed on another drive and$outputPropssupplied uniqueartifact directory names for projects with duplicate names. These are
validation-only settings; no repository build configuration changed.
dotnet test --solution alias-unit-validation.slnx -f net10.0 --no-build --no-restore --artifacts-path $artifacts -p:CustomBeforeDirectoryBuildProps=$outputProps --no-progress --max-parallel-test-modules 2 --ignore-exit-code 8generated with the repository's
New-FilteredSolution.ps1, selectingnet10.0unit tests and excluding samples. Failures are outside the changedprojects: 61 AgentHooks tests could not load
agent_hooks_ffi; oneFileAgentSkillLoader test failed during Windows symlink cleanup with
UnauthorizedAccessException. The exact failed test names match the previouspublished revision. Both affected projects passed in this run.
Integration tests requiring external services were not run.
Related Issue
N/A - maintainer-directed bugfix; no new issue was opened.
AI Assistance
AI assistance details: Copilot assisted with implementation, regression tests,
validation, and this pull request description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.